Skip to content

Code Editor: Improve types and options handling#10900

Closed
westonruter wants to merge 80 commits intoWordPress:trunkfrom
westonruter:fix/wp-code-editor-types-and-options
Closed

Code Editor: Improve types and options handling#10900
westonruter wants to merge 80 commits intoWordPress:trunkfrom
westonruter:fix/wp-code-editor-types-and-options

Conversation

@westonruter
Copy link
Member

@westonruter westonruter commented Feb 10, 2026

This is a follow-up to:

To address:

This is closely related to Core-61175 which introduces PHPStan as the PR for this ticket introduces TypeScript, although currently constrained to apply to files related to the code editor. The pull request introduces TypeScript to help guide the refactor to how linting options are gathered so as to avoid the double-linting issue. Instead of initializing with the default options first and then supplying the WordPress options, now it gathers the WordPress options and supplies them into the code editor when it first initializes.

Additionally, the source files were re-organized to remove our CodeMirror extension code from confusingly being located in a vendor directory.

In subsequent PRs:

  • Remove jQuery dependency
  • Add TypeScript to precommit task and GHA workflow. See Core-64662.

Trac ticket: https://core.trac.wordpress.org/ticket/64661

Gemini Summary

This branch introduces comprehensive TypeScript static analysis for JavaScript files using JSDoc, modernizes several Code Editor components, and refines the build and organizational structure of CodeMirror extensions.

1. TypeScript Static Analysis & Type Safety

  • Infrastructure: Introduced tsconfig.json configured for JS checking (checkJs: true) and strict mode (strict: true). Added a typecheck:js script to package.json to facilitate this analysis.
  • Global Definitions: Created typings/globals.d.ts to provide type definitions for WordPress globals (wp), jQuery, _ (Underscore), Backbone, and HTMLHint.
  • Enhanced Type Definitions:
    • Defined precise JSDoc typedefs in code-editor.js (e.g., CodeEditorInstance, LintingController) to replace generic types.
    • Integrated CodeMirror addon types (lint, show-hint) using triple-slash directives.
    • Eliminated almost all generic any usage in favor of specific intersection types and unions.
  • Strict Checking: Resolved numerous strict-mode issues, including mandatory null checks for optional callbacks and properties.

2. Code Modernization & Refactoring

  • code-editor.js:
    • Modernized variable declarations using const and let.
    • Updated deprecated APIs: replaced event.keyCode with event.key and substr() with slice().
    • Adopted native DOM APIs where appropriate (classList, Node.contains).
    • Standardized code style with trailing commas in multi-line object literals.
  • htmlhint-kses.js:
    • Refactored to modern ES syntax (arrow functions, template literals, and for...of loops).
    • Formatted with Prettier for consistency with modern standards.

3. Organization & Build Process

  • Source Reorganization: Moved WordPress-maintained CodeMirror extensions (htmlhint-kses.js, javascript-lint.js) from vendor/ to lib/ to distinguish them from third-party code.
  • Deprecation: The fakejshint.js file has been documented as deprecated and moved to src/js/_enqueues/deprecated/.
  • Redundancy Cleanup: Removed the redundant esprima.js source file; the build now correctly sources it from the npm package.
  • Linter Optimization:
    • Resolved a double-linting bug during initialization by centralizing option construction.
    • Removed obsolete nested options.options for linting, following modern CodeMirror standards.
  • ESLint Alignment: Added a local .eslintrc.json in lib/codemirror/ to correctly signal ES module parsing for javascript-lint.js.

4. Dependencies

  • Added @types packages for jquery, codemirror, underscore, espree, and htmlhint as devDependencies to support the static analysis effort.

All changes have been verified via npm run typecheck:js and npx grunt jshint:core.

Fixed PhpStorm Inspections

image image

Use of AI Tools

I used Gemini CLI for granular commits, each of which I reviewed.


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

westonruter and others added 4 commits February 10, 2026 15:30
This update replaces all 'var' declarations with 'const' or 'let' and adds the 'eslint-env es6' directive to ensure proper parsing by ESLint.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Updates the JSDoc types from {CodeMirror} to {CodeMirror.Editor} for editor instances to correctly reference the instance interface instead of the namespace. This resolves typing issues in IDEs where methods like getOption() were reported as unresolved.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Replaces the deprecated String.prototype.substr() method with slice() in code-editor.js to adhere to modern JavaScript best practices and resolve editor warnings.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Updates the keyboard event handling in code-editor.js to use the modern event.key property instead of the deprecated event.keyCode, following current web standards.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@github-actions
Copy link

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

westonruter and others added 25 commits February 10, 2026 16:03
Resolves "Referenced UMD global variable" warnings in PhpStorm by passing window._ as a parameter to the IIFE and explicitly typing it with JSDoc.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Fleshes out the CodeEditorSettings type definition with properties for CodeMirror, CSSLint, JSHint, and HTMLHint based on the provided configuration object. Updates all settings parameter references to use this new type for improved editor intelligence.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Resolves type mismatch errors in PhpStorm by marking nested properties in the CodeEditorSettings typedef as optional. This allows empty objects to be assigned to these properties in defaultSettings while maintaining autocompletion support.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Updates CodeEditorInstance to use CodeMirror.EditorFromTextArea and explicitly types local variables in wp.codeEditor.initialize to resolve assignability warnings in PhpStorm. Switched to a deep extend for instanceSettings to ensure nested configuration objects are correctly handled.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Updates the CodeEditorSettings typedef to use the official CodeMirror.EditorConfiguration type for the codemirror property and adds specific string literal types for direction and inputStyle. This resolves type assignability errors in PhpStorm.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Replaces the internal editor.display.wrapper property with the public editor.getWrapperElement() method to resolve unresolved variable warnings in IDEs.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Replaces the jQuery.contains() call with the native DOM element.contains() method. This resolves a parameter mismatch error in some IDEs and leverages modern native APIs.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Replaces the jQuery.hasClass() call with the native classList.contains() method. This follows modern JavaScript practices and removes an unnecessary jQuery dependency for this check.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Replaces the invalid onUpdateLintingOverridden.apply() call with a direct function call. The previous usage incorrectly attempted to pass three arguments to apply() and used the annotations array as the 'this' context.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Updates the JSDoc types for editor parameters in configureLinting and configureTabbing to use CodeMirror.EditorFromTextArea. This resolves unresolved method warnings in PhpStorm for getTextArea() and ensures correct typing for instances created via fromTextArea().

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Updates JSDoc types to include optional performLint and showHint methods provided by CodeMirror addons. Adds a check for performLint existence before calling it to ensure safety and resolve IDE warnings.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Replaces _.filter() with the native Array.prototype.filter() method. This helps PhpStorm correctly infer the type of the annotation object from the LintAnnotation[] array, resolving issues where it was incorrectly identified as a string.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Defines CodeMirrorState and CodeMirrorTokenState typedefs to document properties added by addons (completionActive) and mode-specific states (htmlState, curState). Applies these types to the CodeMirror instance and token variable to resolve unresolved variable warnings in PhpStorm.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Refines the CodeMirrorEditor typedef as a standalone interface with relaxed string signatures for getOption, setOption, on, and off, resolving string assignability warnings in PhpStorm.
- Introduces CodeMirrorSettings to correctly extend EditorConfiguration with addon-specific properties.
- Updates JSDoc for IIFE parameters and internal functions to use these more precise types.
- Adds the 'focused' property to CodeMirrorState.
- Explicitly types the CodeMirror instance in wp.codeEditor.initialize.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Initializes shouldAutocomplete to false and uses boolean coercion for the HTML tag name check. This makes the code more robust and ensures the variable consistent with its intended purpose as a boolean flag.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
… JSDoc.

Add a 'static-analysis:js' script to package.json that runs 'tsc'.
Configure 'tsconfig.json' to check JSDoc in 'src/js/_enqueues/wp/code-editor.js' without emitting files, with settings mirrored from '.jshintrc'.
Add global type definitions in 'typings/globals.d.ts' for 'wp', 'jQuery', '_', and 'Backbone'.
Update 'src/js/_enqueues/wp/code-editor.js' to use global variables directly, satisfying both TypeScript and JSHint.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Break down CodeEditorSettings into nested typedefs (CSSLintRules, JSHintRules, HTMLHintRules) to resolve parsing errors with dot notation in property names.
Rename 'wp.codeEditor~CodeEditorInstance' to 'CodeEditorInstance' to resolve syntax errors caused by the '~' character.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…ings.

- Prefix unused variables '_cm' and '_editor' with an underscore to comply with 'noUnusedLocals' and 'noUnusedParameters'.
- Remove the unused 'CodeMirrorState' typedef.
- Add 'options' property to the 'CodeMirrorEditor' typedef to allow type-safe access to 'codemirror.options.mode'.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Replace 'any' with 'string[]' in the 'gutters' cast for better specificity.
- Use a single direct cast for 'gutters' at declaration.
- Change 'unknown' to 'string' in the 'option' cast for improved readability.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Type 'event' as 'JQuery.MouseDownEvent' to resolve IDE warnings.
- Cast 'event.target' to 'Node' and 'HTMLElement' for safe property access.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Resolve IDE and TypeScript assignment mismatch errors by explicitly casting the returned editor instance to the custom 'CodeMirrorEditor' type.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Integrate CodeMirror 'lint' and 'show-hint' addon types via triple-slash references in 'typings/globals.d.ts'.
- Define 'CodeMirrorState' and refine 'CodeMirrorEditor' typedefs in 'code-editor.js' to provide better IDE resolution for 'completionActive', 'performLint', and 'showHint'.
- Add a type cast in 'getLintOptions' for safe property access on the lint options object.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
westonruter and others added 2 commits February 22, 2026 15:14
Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left some feedback mostly around the TypeScript setup. If you'd like, I'm happy to work on adjustments and push changes.

Frankly, I'd like to start using TypeScript syntax directly for these files that are already being compiled/bundled where we're adding a lot of type information. Whether a file is TypeScript or not could also serve as the condition for whether to check types.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the file diff between fdafcd3
and 5f54c0b. No obvious issues in the diff 👍

diff --git o/src/js/_enqueues/vendor/codemirror/htmlhint-kses.js w/src/js/_enqueues/lib/codemirror/htmlhint-kses.js
index 1aa7ffbd08..e08c4c0f5b 100644
--- o/src/js/_enqueues/vendor/codemirror/htmlhint-kses.js
+++ w/src/js/_enqueues/lib/codemirror/htmlhint-kses.js
@@ -1,30 +1,47 @@
 /* global HTMLHint */
-/* eslint no-magic-numbers: ["error", { "ignore": [0, 1] }] */
-HTMLHint.addRule({
+/* eslint no-magic-numbers: ["error", { "ignore": [1] }] */
+HTMLHint.addRule( {
 	id: 'kses',
 	description: 'Element or attribute cannot be used.',
-	init: function( parser, reporter, options ) {
+
+	/**
+	 * Initialize.
+	 *
+	 * @this {import('htmlhint/types').Rule}
+	 * @param {import('htmlhint').HTMLParser} parser - Parser.
+	 * @param {import('htmlhint').Reporter} reporter - Reporter.
+	 * @param {Record<string, Record<string, boolean>>} options - KSES options.
+	 * @return {void}
+	 */
+	init: function ( parser, reporter, options ) {
 		'use strict';
 
-		var self = this;
-		parser.addListener( 'tagstart', function( event ) {
-			var attr, col, attrName, allowedAttributes, i, len, tagName;
-
-			tagName = event.tagName.toLowerCase();
+		parser.addListener( 'tagstart', ( event ) => {
+			const tagName = event.tagName.toLowerCase();
 			if ( ! options[ tagName ] ) {
-				reporter.error( 'Tag <' + event.tagName + '> is not allowed.', event.line, event.col, self, event.raw );
+				reporter.error(
+					`Tag <${ event.tagName }> is not allowed.`,
+					event.line,
+					event.col,
+					this,
+					event.raw
+				);
 				return;
 			}
 
-			allowedAttributes = options[ tagName ];
-			col = event.col + event.tagName.length + 1;
-			for ( i = 0, len = event.attrs.length; i < len; i++ ) {
-				attr = event.attrs[ i ];
-				attrName = attr.name.toLowerCase();
-				if ( ! allowedAttributes[ attrName ] ) {
-					reporter.error( 'Tag attribute [' + attr.raw + ' ] is not allowed.', event.line, col + attr.index, self, attr.raw );
+			const allowedAttributes = options[ tagName ];
+			const column = event.col + event.tagName.length + 1;
+			for ( const attribute of event.attrs ) {
+				if ( ! allowedAttributes[ attribute.name.toLowerCase() ] ) {
+					reporter.error(
+						`Tag attribute [${ attribute.raw }] is not allowed.`,
+						event.line,
+						column + attribute.index,
+						this,
+						attribute.raw
+					);
 				}
 			}
-		});
-	}
-});
+		} );
+	},
+} );

@westonruter
Copy link
Member Author

@sirreal:

I've left some feedback mostly around the TypeScript setup. If you'd like, I'm happy to work on adjustments and push changes.

Please do! Please amend as you see fit. I'm definitely not a TypeScript expert, so your expertise is much appreciated.

Frankly, I'd like to start using TypeScript syntax directly for these files that are already being compiled/bundled where we're adding a lot of type information.

That sounds good to me. I think in this PR that would mean we could turn src/js/_enqueues/lib/codemirror/javascript-lint.js and tools/vendors/codemirror-entry.js into TypeScript files. The other JS files, namely src/js/_enqueues/lib/codemirror/htmlhint-kses.js and src/js/_enqueues/wp/code-editor.js I think should remain as JS since they should be able to be run directly in the browser. (For example, I symlink them to src/wp-includes to avoid having to run a build when I make changes, although I'm willing to concede if this is just me.)

Whether a file is TypeScript or not could also serve as the condition for whether to check types.

There is a lot of type information added to code-editor.js though, so we should be sure to add that as an allow-list of JS files that we also start type-checking. I really would like to start incrementally adding more and more of the core JS files to beying type-checked as well.

Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pushed several smaller changes mostly to adjust the TypeScript project configuration.

This seems ready. Thanks!

@sirreal
Copy link
Member

sirreal commented Mar 3, 2026

The typcheck:js script doesn't run on CI, does it? That would be a good follow-up.

@westonruter
Copy link
Member Author

Thank you!

The typcheck:js script doesn't run on CI, does it? That would be a good follow-up.

Yes, that is what Core-64662 for.

* @since 4.9.0
*
* @param {string|jQuery|Element} textarea - The HTML id, jQuery object, or DOM Element for the textarea that is used for the editor.
* @param {string|Element} textarea - The HTML id, jQuery object, or DOM Element for the textarea that is used for the editor.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sirreal This change seems like it should be undone. In fact, a jQuery object is expected to be able to be passed in here:

editor = wp.codeEditor.initialize( $( '#newcontent' ), codeEditorSettings );

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted in 5192ff9

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine. This was flagged as an error for me when I tried tsgo, but the stable tsc does not complain.

Copy link
Member

@sirreal sirreal Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed ed9f7b0 that I believe corrects the jQuery types and this type.

The wp.codeEditor.initialize() function is currently passed a jQuery object in core.

This reverts commit 9338964.
@github-actions
Copy link

github-actions bot commented Mar 3, 2026

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props westonruter, jonsurrell, justlevine.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

pento pushed a commit that referenced this pull request Mar 3, 2026
…inting at initialization.

* Refactor how CodeMirror is initialized so that the full settings are provided up-front. This avoids the linting from being applied twice at initialization, the first time with an incorrect configuration.
* Add initial TypeScript configuration for core with `npm run typecheck:js`.
* Add comprehensive types for code editor files: `code-editor.js`, `javascript-lint.js`, and `htmlhint-kses.js`.
* Move code editor scripts from `src/js/_enqueues/vendor/codemirror/` to `src/js/_enqueues/lib/codemirror/`. The CodeMirror library is sourced from the npm package as of r61539.
* Remove (deprecated) `esprima.js` from being committed to SVN since in r61539 it was switched to using the npm package as its source.
* Move `fakejshint.js` to `src/js/_enqueues/deprecated`.

Developed in #10900

Follow up to r61611, r61539.

Props westonruter, jonsurrell, justlevine.
See #64662, #48456.
Fixes #64661.


git-svn-id: https://develop.svn.wordpress.org/trunk@61800 602fd350-edb4-49c9-b593-d223f7449a82
@github-actions
Copy link

github-actions bot commented Mar 3, 2026

A commit was made that fixes the Trac ticket referenced in the description of this pull request.

SVN changeset: 61800
GitHub commit: b6c1bb7

This PR will be closed, but please confirm the accuracy of this and reopen if there is more work to be done.

@github-actions github-actions bot closed this Mar 3, 2026
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Mar 3, 2026
…inting at initialization.

* Refactor how CodeMirror is initialized so that the full settings are provided up-front. This avoids the linting from being applied twice at initialization, the first time with an incorrect configuration.
* Add initial TypeScript configuration for core with `npm run typecheck:js`.
* Add comprehensive types for code editor files: `code-editor.js`, `javascript-lint.js`, and `htmlhint-kses.js`.
* Move code editor scripts from `src/js/_enqueues/vendor/codemirror/` to `src/js/_enqueues/lib/codemirror/`. The CodeMirror library is sourced from the npm package as of r61539.
* Remove (deprecated) `esprima.js` from being committed to SVN since in r61539 it was switched to using the npm package as its source.
* Move `fakejshint.js` to `src/js/_enqueues/deprecated`.

Developed in WordPress/wordpress-develop#10900

Follow up to r61611, r61539.

Props westonruter, jonsurrell, justlevine.
See #64662, #48456.
Fixes #64661.

Built from https://develop.svn.wordpress.org/trunk@61800


git-svn-id: http://core.svn.wordpress.org/trunk@61106 1a063a9b-81f0-0310-95a4-ce76da25c4cd
@westonruter
Copy link
Member Author

The typcheck:js script doesn't run on CI, does it? That would be a good follow-up.

Ready for review: #11131

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants